feat: Add Pytest CI mode to optimise migrations #3815
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Uffizzi Preview |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3815 +/- ##
=======================================
Coverage 95.91% 95.92%
=======================================
Files 1122 1122
Lines 35482 35501 +19
=======================================
+ Hits 34034 34053 +19
Misses 1448 1448 ☔ View full report in Codecov by Sentry. |
khvn26
requested review from
a team and
novakzaballa
and removed request for
a team
| def pytest_configure(config: pytest.Config) -> None: | ||
| if ( | ||
| config.option.ci | ||
| and config.option.dist != "no" |
There was a problem hiding this comment.
I'm surprised that the option's value comes out as "no" instead of False.
There was a problem hiding this comment.
"no" is, in fact, one of xdist's distribution modes. See here.
| if ( | ||
| config.option.ci | ||
| and config.option.dist != "no" | ||
| and not hasattr(config, "workerinput") |
There was a problem hiding this comment.
It seems weird that if ci is present that if workerinput is set that this will silently fail. Don't we prefer an exception if ci is set and so is workerinput?
There was a problem hiding this comment.
Having ci and having workerinput is a legitimate case when we need this hook to be a noop.
|
|
||
| def pytest_addoption(parser: pytest.Parser) -> None: | ||
| parser.addoption( |
There was a problem hiding this comment.
Just confirming that this function will be auto-invoked without a decorator.
There was a problem hiding this comment.
See coverage report :)
Dive into pytest hooks if you're curious about how this works.
| def django_db_setup(request: pytest.FixtureRequest) -> None: | ||
| if ( | ||
| request.config.option.ci | ||
| and (xdist_worker_id_suffix := get_xdist_worker_id(request)[2:]).isnumeric() |
There was a problem hiding this comment.
It's not obvious why
get_xdist_worker_id(request)[2:]That part of the logic ends with a [2:]. Can you add a comment?
khvn26
force-pushed
the
feat/pytest-migrate-once
branch
from
42ff7ac to
dd2d8f7
Compare
khvn26
force-pushed
the
feat/pytest-migrate-once
branch
from
dd2d8f7 to
04e19dd
Compare
Thanks for submitting a PR! Please check the boxes below:
pre-committo check lintingdocs/if required so people know about the feature!Changes
This PR introduces a "CI mode" to Core API test runner, invoked by providing the
--cioption to pytest.In this mode, the following optimisations are applied to how pytest manages Django databases when in xdist mode:
CREATE DATABASE ... WITH TEMPLATEquery.Locally, this yields a substantial decrease in overall running time:
I suspect it won't be as prominent in CI as it probably improves linearly with the number of workers (which is going to be less in CI).
How did you test this code?
The only proper way was to run the whole test suite.